CI: build the docs for pull requests into develop as well - #1104
Conversation
`branches` filters on a pull request's base, and almost every PR here is opened against develop: 25 of the last 30, with 3 against master. So the build that runs with -W --keep-going saw a docs change for the first time in a release batch, well away from whatever caused it. The path filter already keeps this off PRs that cannot affect the docs, and `rocketpy/**` is in it because docstrings feed the autodoc reference, so a docstring edit merged into develop could break the API pages unnoticed too. `push` is left on master alone. The pull request check is where the feedback is worth having, and running both would double the cost for a second opinion on the same commit. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1104 +/- ##
===========================================
+ Coverage 82.18% 82.55% +0.37%
===========================================
Files 122 128 +6
Lines 16355 16555 +200
===========================================
+ Hits 13441 13667 +226
+ Misses 2914 2888 -26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The new trigger has now exercised itself: I have corrected four things in the description that claimed more than the change delivers.
I had called this "a gate". It produces a check; whether anything blocks on it is a branch rule rather than this file. Worth adding that this workflow is not in a shape to be made required as it stands, because a top-level I had written that "nothing reaches the network". The documentation examples make no live calls, which is the part that matters, but the job still does The validation section described a local build that had to exclude the notebooks for want of pandoc. The real run has since made that redundant, so it now cites that instead. |
1 participant
Addresses #1099. Not
Closes, because the closing keyword only fires when the pull request targets the default branch, and this one targetsdevelop. #1094 and #1096 are both still open for exactly that reason.Pull request type
Checklist
One workflow file changes and nothing under
rocketpy/ortests/does, so no code test was added. The Tests, Linters and Documentation workflows all pass on the current head.Current behavior
branchesfilters on the pull request's base, and almost nothing here targetsmaster. Of the last 30 pull requests, 25 went todevelopand 3 tomaster. So the build that runs with-W --keep-goingfirst sees a docs change whendevelopis promoted, in a batch, a long way from whatever caused it.#1097 is the case that prompted this. It changed
docs/user/custom_sampler.rst, targeteddevelop, and its only check was the auto-assign workflow. My localsphinx-buildwas the only thing standing behind the reStructuredText, which is not where that check belongs.The path filter includes
rocketpy/**, because docstrings feed the autodoc reference, so this is not only aboutdocs/. A docstring edit merged intodevelopcan break the API pages with nothing to say so.New behavior
pushis deliberately left onmasteralone. The pull request check is where the feedback is useful, and running both would pay twice for a second opinion on the same commit.Breaking change
Additional information
Two things worth knowing before adding a
-Wbuild to every docs-affecting pull request. A check rather than a gate: whether it can block a merge is a branch-rule decision, not this file's, and a workflow with top-levelpathsfilters is the wrong shape to make required as it stands.It is not slow. The job sets
DOCS_SKIP_EXECUTE: "1", so notebooks are not executed andjupyter-executecells render as static blocks. The documentation examples therefore make no live weather or external-data calls, which is the expensive and flaky part; the job still installs pandoc and the Python dependencies like any other. The run on this pull request took 2m27s.developbuilds clean today, so this does not switch on a check that is already red. This pull request proves it rather than arguing it:.github/workflows/docs.ymlis inside the workflow's ownpathslist, so the change triggered itself.Documentation / build-docs (3.12)ran against the merge ref and the warnings-as-errors build finished clean.The workflow asks for
contents: readand uses no repository or organization secrets, so a fork pull request runs it with a read-only token, subject to whatever approval policy the repository applies to outside contributors. That is where it is needed most.It has to stay on
pull_requestrather thanpull_request_target: it checks out the merge tree and then installs and imports the package,docs/conf.pyand the Sphinx extensions from it, all of which the pull request controls.